Implement ABBA-based analysis for chop-and-nod observations#221
Conversation
|
There was a problem hiding this comment.
Thank you for implementing this feature. Your code assumes that the string abba exists somewhere in an observation name but this is not correct. I think you should update pswsc function instead of making a new one.
Should I add a scan time lapse plot like the figure outputted using pswsc function?
For the reason above, we don't need to make a new plotting code but simply replace the current spectrum made by the ABAB method with that by the ABBA method.
|
Minor comments:
|
|
|
I will add changing nod pair code in this issue. So please do not close. |
There was a problem hiding this comment.
Thank you for the update. Here are minor comments:
- Please sort constants alphabetically and use a plural form for the ABBA phases.
- Please add
subtract_per_abba_cycleabovesubtract_per_scan(i.e. sort alphabetically). - Description of
subtract_per_abba_cyclelooks not correct. - Please rename
subtract_per_scanbecause now it is not used per scan but per ABBA phase. ABBA_PHASE == set(...)(L1177) certainly works but it is not a good coding practice: A comparison (a constant to be compared) should usually put on the right-hand side. In addition,.valuesdoes not seem necessary in this case.
|
Related to 5., it would be great to check out a coding style called "early returns" or "guard clauses". For example, def subtract_per_abba_cycle(dems: xr.DataArray, /) -> xr.DataArray:
if not set(np.unique(dems.abba_phase)) == ABBA_PHASES:
return xr.DataArray(np.nan)
return (
dems
.groupby("abba_phase")
.map(subtract_per_abba_phase)
.mean("abba_phase")
) |
|
I changed following Taniguchi-san's comment and reformatted the code. |
|
change explanation of |
ShinjiFujita
left a comment
There was a problem hiding this comment.
I did not find any problems.
|
@ShinjiFujita san, thank you for your checking. I will merge the PR and make a new public release soon. By the way, my apologies for keeping your PR #219 still open... I will check it soon as well. |
Add ABBA method code #220
Should I add a scan time lapse plot like the figure outputted using pswsc function?